Fix transit import/export of hmac-only keys #20864
Conversation
cipherboy
added
bug
| @@ -1585,7 +1590,7 @@ func (p *Policy) RotateInMemory(randReader io.Reader) (retErr error) { | |||
| if p.Type == KeyType_AES128_GCM96 { | |||
| numBytes = 16 | |||
| } else if p.Type == KeyType_HMAC { | |||
| numBytes := p.KeySize | |||
| numBytes = p.KeySize | |||
There was a problem hiding this comment.
This shadowing was causing HMAC keys to be shorter than expected.
When initially implemented, exporting HMAC keys resulted in returning the unused, internal HMACKey value rather than the main Key value that is used for HMAC operations. This is a breaking change. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When generating HMAC-typed keys, set HMACKey = Key consistently, to allow users of HMAC-typed keys to use them backwards compatibly. Notably, this could discard the (unused) HMACKey field set today. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-fix-transit-import-export-hmac-keys
branch
from
f0ca27b to
b0935e6
Compare
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-fix-transit-import-export-hmac-keys
branch
from
b0935e6 to
513eef5
Compare
| @@ -0,0 +1,5 @@ | |||
| ```release-note:bug | |||
| secrets/transit: Fix export of HMAC-only key, correctly exporting the key used for sign operations. For consumers of the previously incorrect key, use the plaintext export to retrieve these incorrect keys and import them as new versions. | |||
There was a problem hiding this comment.
FYI - these didn't format properly in the changelog. I think you might need a different backtick section for each bullet. Can you check the docs? Don't fix this file, just for next time.
There was a problem hiding this comment.
Interesting, thank you! I thought I had seen an example like this, but maybe it was also fixed; perhaps we should fix this one, so that we know its not correct?
https://github.com/hashicorp/vault/blob/main/changelog/README.md doesn't really give any indication of how to format changelog files with multiple impacts, probably worth calling out, tbh.
There was a problem hiding this comment.
Ah, the upstream docs do call it out: https://github.com/hashicorp/go-changelog#change-file-formatting :
Sometimes PRs have multiple changelog entries associated with them. In this case, use multiple blocks.
Fundamentally the second commit is the important one: we need to ensure HMAC-only keys exposes the same interface as non-HMAC keys, that is, the HMAC key is in
.HMACKeyand the "real" key is in.Key-- but in our case, HMAC should be the same value across both in case anyone is using the field directly in applications.The first is more or less a duplicate, for backwards compatibility: we won't nuke the existing HMAC keys with different-HMACKey-field-values, so we'll want to export the real one under
export/hmac-keys/.... For anyone who inadvertently used the wrong HMAC key, they can use a plaintext backup (and/orsys/raw) to import the separate, incorrect versions as proper versions into this key.Related: #20804
Jira: VAULT-16702